Added cardano connector plugin #1636
Conversation
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1636 +/- ##
========================================
Coverage 99.95% 99.96%
========================================
Files 339 342 +3
Lines 29782 30457 +675
========================================
+ Hits 29770 30445 +675
Misses 8 8
Partials 4 4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
| func (c *Cardano) GenerateErrorSignature(ctx context.Context, event *fftypes.FFIErrorDefinition) string { | ||
| // TODO: impl | ||
| return "" | ||
| } |
There was a problem hiding this comment.
Would be interested to understand more how errors are reported in Cardano from smart contract
There was a problem hiding this comment.
The cardano smart contract doesn't report errors right now, only events. I don't understand the difference between errors and events, and I couldn't find any code in other plugins which streamed errors from the connector.
I've copied the "event signature" code into this function, but that's just a guess at what it's supposed to do. I'd want to see how consumers actually use events to be more confident about that.
| if !isBatch { | ||
| var receipt common.BlockchainReceiptNotification | ||
| _ = json.Unmarshal(msgBytes, &receipt) | ||
|
|
||
| err := common.HandleReceipt(ctx, "", c, &receipt, c.callbacks) | ||
| if err != nil { | ||
| l.Errorf("Failed to process receipt: %+v", msgTyped) | ||
| } | ||
| } |
There was a problem hiding this comment.
I see that this is heavily inspired from FFTM and the evm plugin - I would recommend in the future moving to a durable event stream here where the receipts have to be ack'd back as part of a batch.
There was a problem hiding this comment.
I was using EVM, Tezos, and FFTM as references for how the eventstream is supposed to work (trying my best to make Cardano match the semantics of other connectors). It looked like other connectors assume that every message in a batch is an event, not a receipt.
I think it makes sense to use resilient ack-ing for everything, but I'm a little wary about inventing semantics for it in this plugin that nothing else is using.
There was a problem hiding this comment.
Agreed - probably as a follow up. I'm building a framework at moment to allow for resilient ack-ing and get confirmation from FireFly core it has processed things correctly
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
There was a problem hiding this comment.
Was reminded yesterday of the key being mandatory today as part of deploying a contract and it doesn't make sense for Cardano as the connector has used that API to run a off-chain dApp that is responsible of building Cardano TX and indexing state. Raised a discussion on Discord https://discord.com/channels/905194001349627914/928377875827154984/1349704667524763689
|
|
||
| ## Create the stack | ||
|
|
||
| A Cardano stack can be run in two different ways; with a firefly |
There was a problem hiding this comment.
| A Cardano stack can be run in two different ways; with a firefly | |
| A Cardano stack can be run in two different ways with Hyperledger FireFly |
|
Sorry I tagged a comment in a linked PR as |
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
|
Timeout error in a Cardano unit test |
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
There was a problem hiding this comment.
@SupernaviX This is looking really good! A few small comments, thanks for the detailed docs :)
|
|
||
| CONTRACT_PATH="/path/to/your/dapp" | ||
| FIREFLY_URL="http://localhost:5000" | ||
| firefly-cardano-deploy --contract-path "$CONTRACT_PATH" --firefly-url "$FIREFLY_URL" |
There was a problem hiding this comment.
This is great, probably as a follow up adding this to the FireFly CLI would be great :) We have similar deploy and install commands in for Fabric and EVM
There was a problem hiding this comment.
That would definitely be more convenient than asking users to install some standalone tool. But part of deploying these contracts is compiling rust code to webassembly. Would it be OK for the FireFly CLI to shell out to cargo?
There was a problem hiding this comment.
oh that is good point, as all these tools are in Rust! I think it would be fine
| - Start sending events | ||
| - For the Subscription named `sample-contract` | ||
| - On the `default` namespace | ||
| - Automatically "ack" each event which will let FireFly immediately send the next event when available |
There was a problem hiding this comment.
We recommend not using autoack as it's safer for application to manual ack once they have processed the event
There was a problem hiding this comment.
That's what the other contract readmes say to do. Are there instructions for how to consume this kind of API without autoack somewhere?
There was a problem hiding this comment.
Yes, there should be in the subscription section - let me pull it out
There was a problem hiding this comment.
There was a problem hiding this comment.
It does need some cleaning up maybe
There was a problem hiding this comment.
I added a note about autoack below this. I do think autoack behavior makes sense for initial setup/experimentation, as long as users know not to leave it on.
internal/networkmap/did.go
Outdated
| func (nm *networkMap) generateCardanoAddressVerifier(identity *core.Identity, verifier *core.Verifier) *VerificationMethod { | ||
| return &VerificationMethod{ | ||
| ID: verifier.Hash.String(), | ||
| Type: "PaymentVerificationKeyShelley_ed25519", // hope that it's safe to assume we always use Shelley |
| "cardanoconnect": { | ||
| "image": "ghcr.io/hyperledger/firefly-cardanoconnect", | ||
| "tag": "v0.4.1", | ||
| "sha": "78b1008bd62892f6eda197b5047d94e61621d0f06b299422ff8ed9b34ee5ce50" | ||
| }, | ||
| "cardanosigner": { | ||
| "image": "ghcr.io/hyperledger/firefly-cardanosigner", | ||
| "tag": "v0.4.1", | ||
| "sha": "d0b76613ccc70ff63e68b137766eb009d589489631cee6aabf2b45e33a1ca5d3" | ||
| }, |
There was a problem hiding this comment.
This might need updating again
There was a problem hiding this comment.
These are the latest published versions, I just confirmed that with manifestgen.sh. We could probably cut new versions of those soon, but I'd rather not block this big long-lived PR on it.
|
I think it needs an |
Probably, but it's also failing due to the crypto audit. So I'm just waiting until the build passes on main, so that I can merge main and get a working build (and then address your comments). |
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Signed-off-by: Simon Gellis <[email protected]>
Proposed changes
Adds a cardano connector plugin to firefly core. This connector interacts with the firefly-cardanoconnect service from firefly-cardano.
Types of changes
Please make sure to follow these points
< issue name >egAdded links in the documentation.Other Information
The Cardano connector and implementation is still a work in progress; in particular we need documentation and tests. However, it works well enough to demo, and I'd like to merge this relatively soon so that people can run it without building firefly-core from a branch, or downloading an image from a private repo.